Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #4537 +/- ##
==========================================
- Coverage 92.32% 92.32% -0.01%
==========================================
Files 371 371
Lines 12198 12196 -2
==========================================
- Hits 11262 11260 -2
Misses 936 936 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@MattWestb Do you remember why you added this attributes to the output side of this cluster? The rotation events seem to come from the output cluster, but the attribute data is on the input cluster. Feel like we should remove those attributes from the output cluster (TuyaSmartRemoteOnOffCluster) |
|
This looks like it should not be a complete OnOff cluster on input here, since not all attributes are supported on input and we end up creating a switch entity. |
|
This will depend on zigpy/zha#590 |
|
Hej du glade !! Im not one code worrier and more "copy and paste / try and fail" but we was starting with the dimmer switch that is only having the short and long press commands but it was one long road getting there. Next was the rotating knob that we was decoding and only adding the new commands it was sending and it was looking working OK but no deep diving so more then likely it can being in wrong direction like tuya oft have done. In the end the deleting the On/Of in cluster so THA is not doing one light in the GUI. I have one dimmer switch and one rotating knob so can doing testing and sniffing if you need more info for debugging the problem. |
|
Anyway you can test reading writing of the switch mode on any of the devices you have? And see if you get any data from that attribute when it's on output endpoint? I cant get that to work on any of the ts004f. (I would like to remove it from the output cluster if possible) |
|
Was reading all attributes on Clusters: Edit: Writing 0 or 1 say OK but no working mod not changed on the device and no attribute changed in ZHA log. |
|
Good then it looks like it works same as I see it. If you also add the onoff cluster or the attribute cluster on input side it works. (The events still must be on output side) Maybe there was some compatibility that switches direction of a cluster command that was removed (the warning that is logged). |
|
The TS004X devices is having the same defect (and some other tuya devices) so i think they is also have lost some functionality from ZHA but still working on the device then its possible doing it. |
These attributes are not available on the output side, only on the input side. This fixes warnings about direction being wrong on the onoff cluster, since the onoff cluster was removed on the input side.
The cluster is a normal On/Off cluster with some extensions.
0ad7af7 to
fbc4627
Compare
| TURN_OFF, | ||
| TURN_ON, | ||
| ) | ||
| from zhaquirks.quirk_ids import TUYA_PLUG_ONOFF |
There was a problem hiding this comment.
I dont understand the logic of this naming then the tuya scene commands is only being used of scene remotes and no plugs what i knowing.
There was a problem hiding this comment.
This may not be needed anymore. I think i originally used it to match some cluster handler.
Forgot this was not i draft. Will remove that change.
There was a problem hiding this comment.
Actually.. Some way to mark the cluster having the "switch_mode" attribute is needed. The name of the quirk is bad thou.
The OnOff cluster have extensions. It's still a standard OnOff cluster on both client and server side.
Also I don't think we need to have TuyaSmartRemoteOnOffCluster and TuyaZBOnOffAttributeCluster. They are really only the server and the client side of same cluster. All other clusters are combined to describe both sides.
There was a problem hiding this comment.
Most tuya quirk is in the pipe for being converted to V2 but its great that you is digging it it so some is knowing how the device is working and can getting the code catching it.
Sadly tuya is doing there "zigbee" implantation that is mostly OK but have making very bad shortcuts like on this devices is not supporting binding in the normal way (binding cluster to devices and / or groups) instead adding ther as lights that is out of all standards.
And thanks for your work done !!
There was a problem hiding this comment.
Most logic is renaming the TUYA_PLUG_ONOFF if its being used for other device types then plugs (the first use of the old cluster was for the TS004F and then extended for the plugs and adding the back light and children lock).
|
All TS004F devices is needed this for understanding the "scene commands". zha-device-handlers/zhaquirks/tuya/__init__.py Lines 1048 to 1060 in 04c61bb Plus the now not working attribute you is fixing here. Only look what is needed then i remember the device is not sending commands in the right direction but if you can getting it working without the warnings its being great. The quirks naming was from the start from the tuya is naming its devices that can being found in there dev docks but many have doing new quirks with other names / meanings. |
This is just two sides of the same cluster and such are normally handled as a single cluster definition.
|
How about this change? I merged the two OnOff cluster's since it was just client side and server side of same extensions and regenerated the test cases in zha and it only render very minor changes for the testcases there. Ps. The scene stuff there are just standard server commands sent same as any other levelcontrol/onoff, just a manufacture specific extension to the OnOff. Ie they are not doing anything strange with those. Can easily be handled as event entities once we have those changes in. |
Proposed change
Restore OnOff cluster on TS004F:
These attributes are not available on the output side, only on the input side.
This fixes warnings about direction being wrong on the onoff cluster, since
the onoff cluster was removed on the input side.
Restore standard name on the TuyaSmartRemoteOnOffCluster cluster.
This is still a standard On/Off cluster, it just have some extensions.
Additional information
I have tested reading of the switch_mode attribute from this, which now works as expected.
Depends on: zigpy/zha#591 to ensure the OnOff cluster does not end
up adding a switch entity.
Depends on: zigpy/zha#590 to make sure this is added as a select entity
Device diagnostics
Checklist
pre-commitchecks pass / the code has been formatted using Black